Changes made in configuration.py to accept environmental variables #2390

p172913 · 2025-04-29T06:03:02Z

What type of PR is this?
/kind bug

What this PR does / why we need it:
This PRs will read environment variables assigned for proxy and no_proxy.

Which issue(s) this PR fixes:
Fixes #2321

Special notes for your reviewer:
Unit test is added kubernetes\base\stream\ws_client_test.py

Does this PR introduce a user-facing change?
None

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:
None

k8s-ci-robot · 2025-04-29T06:03:05Z

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

p172913 · 2025-04-29T18:01:06Z

@roycaihw , @fabianvf Please approve my PR

What type of PR is this? /kind bug What this PR does / why we need it: This PRs will read environment variables assigned for proxy and no_proxy.

Update configuration.py What type of PR is this? /kind bug What this PR does / why we need it: This PRs will read environment variables assigned for proxy and no_proxy.

p172913 · 2025-05-01T19:14:29Z

@roycaihw , @fabianvf Please review my PR...

p172913 · 2025-05-05T15:50:20Z

@roycaihw , @fabianvf Please review my PR...

roycaihw · 2025-05-07T20:04:00Z

/assign

roycaihw · 2025-05-07T22:44:26Z

kubernetes/client/configuration.py

+        if(os.getenv("HTTPS_PROXY")):self.proxy=os.getenv("HTTPS_PROXY")
+        if(os.getenv("https_proxy")):self.proxy=os.getenv("https_proxy")
+        if(os.getenv("HTTP_PROXY")):self.proxy=os.getenv("HTTP_PROXY")
+        if(os.getenv("http_proxy")):self.proxy=os.getenv("http_proxy")


This file is generated by the upstream openapi generator. Could you make the change upstream?

I have checked https://github.com/OpenAPITools/openapi-generator. We need to change in https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/resources/python/configuration.mustache (as it is python code change.). But while I am at it. I observed that there is no no_proxy variable there.

I am not sure whether I can add no_proxy variable to it or not. It was removed for some reasons according to this issue OpenAPITools/openapi-generator#20226.

If it is only for self.proxy=None. I can add it to https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/resources/python/configuration.mustache.

I observed that there is no no_proxy variable there.

This client is based on an old version of the upstream generator. The upstream generator probably had some change to the no_proxy variable since then.

Instead of upstreaming the changes can we downstream the changes to files that using configuration.py variables. i.e. python_kubernetes\kubernetes\base\stream\ws_client.py and python_kubernetes\kubernetes\client\rest.py. We can add this change in above files. But I have a doubt will it impact this changes if we use Open-api generator...

Hmm not sure. From the title "Merge branch 'kubernetes-client:master' into master" it sounds like it was trying to merge the upstream master into your master branch.

An alternative is to open a new PR with a fresh branch, if you cannot delete this commit

Even I cann't delete it. It is not my commit.

@roycaihw Can you check and approve my PR

@roycaihw Can you check and approve my PR

@roycaihw Can you check and approve my PR

Even though this client is using a old version of the generator, can we first get this change accepted by the upstream generator, and then we can patch this client?

k8s-ci-robot · 2025-05-17T14:28:40Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: p172913
Once this PR has been reviewed and has the lgtm label, please ask for approval from roycaihw. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

This reverts commit b295c2d.

roycaihw

Please remove the "Merge branch 'kubernetes-client:master' into master" commit to make the commits clear. You may have to start from a new branch to do so.

roycaihw · 2025-05-27T20:02:42Z

kubernetes/client/configuration.py

+        if(os.getenv("HTTPS_PROXY")):self.proxy=os.getenv("HTTPS_PROXY")
+        if(os.getenv("https_proxy")):self.proxy=os.getenv("https_proxy")
+        if(os.getenv("HTTP_PROXY")):self.proxy=os.getenv("HTTP_PROXY")
+        if(os.getenv("http_proxy")):self.proxy=os.getenv("http_proxy")


If both HTTPS_PROXY and HTTP_PROXY are set, does this mean HTTP_PROXY will override HTTPS_PROXY? Is this behavior matching the websocket client behavior?

If both HTTPS_PROXY and HTTP_PROXY are set, HTTP_PROXY will override HTTPS_PROXY. But Web client has only one self.proxy variable. So, I think it will work fine unless user provide both HTTPS_PROXY and HTTP_PROXY. If user needed both variables we need to check that condition. Even then we can just give HTTPS_PROXY priority...

If user need to declare both HTTPS_PROXY and HTTP_PROXY. He need to mention which proxy he is using.

If user declared both HTTPS_PROXY and HTTP_PROXY and he didn't mention then it will choose latest declared.

If we need to give HTTPS_PROXY priority over HTTP_PROXY. Then I need to change the code accordingly....

roycaihw · 2025-05-27T20:03:13Z

kubernetes/client/configuration.py

        """Proxy URL
        """
        self.no_proxy = None
+        if(os.getenv("NO_PROXY")):self.no_proxy=os.getenv("NO_PROXY")
+        if(os.getenv("no_proxy")):self.no_proxy=os.getenv("no_proxy")


Similar question here. Does no_proxy override NO_PROXY if both of them are set?

Yes no_proxy override NO_PROXY. But this is for case sensitive environments like macOS . Only if user declared both no_proxy and NO_PROXY. In windows it will treat both as same. So, I added both no_proxy and NO_PROXY. But they are same...

roycaihw · 2025-05-27T20:04:11Z

kubernetes/client/configuration.py

+        if(os.getenv("HTTPS_PROXY")):self.proxy=os.getenv("HTTPS_PROXY")
+        if(os.getenv("https_proxy")):self.proxy=os.getenv("https_proxy")
+        if(os.getenv("HTTP_PROXY")):self.proxy=os.getenv("HTTP_PROXY")
+        if(os.getenv("http_proxy")):self.proxy=os.getenv("http_proxy")


Even though this client is using a old version of the generator, can we first get this change accepted by the upstream generator, and then we can patch this client?

This reverts commit 638abe2, reversing changes made to 6b85a93.

p172913 · 2025-05-28T10:39:07Z

Please remove the "Merge branch 'kubernetes-client:master' into master" commit to make the commits clear. You may have to start from a new branch to do so.

This commit is reverted back.

p172913 · 2025-05-28T10:56:02Z

I added a bash script file. This will edit configuration.py file if we create it using new opneapi generator.

k8s-ci-robot requested review from fabianvf and roycaihw April 29, 2025 06:03

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Apr 29, 2025

p172913 added 2 commits April 30, 2025 16:31

Update configuration.py

afd1d44

What type of PR is this? /kind bug What this PR does / why we need it: This PRs will read environment variables assigned for proxy and no_proxy.

Update ws_client_test.py

8f356ef

Update configuration.py What type of PR is this? /kind bug What this PR does / why we need it: This PRs will read environment variables assigned for proxy and no_proxy.

p172913 force-pushed the master branch from a14e698 to 8f356ef Compare April 30, 2025 11:03

k8s-ci-robot removed the do-not-merge/invalid-commit-message Indicates that a PR should not merge because it has an invalid commit message. label Apr 30, 2025

k8s-ci-robot assigned roycaihw May 7, 2025

roycaihw reviewed May 7, 2025

View reviewed changes

Update configuration.py

6b85a93

Merge branch 'kubernetes-client:master' into master

638abe2

p172913 force-pushed the master branch from 638abe2 to afd1d44 Compare May 20, 2025 14:05

p172913 added 4 commits May 20, 2025 22:56

Added

b778952

Added insert_proxy_config.sh to edit configuration.py in client

b295c2d

Revert "Added insert_proxy_config.sh to edit configuration.py in client"

9c35894

This reverts commit b295c2d.

To avoid condition self.no_proxy is already present

1c69346

p172913 requested a review from roycaihw May 27, 2025 07:00

roycaihw reviewed May 27, 2025

View reviewed changes

Revert "Merge branch 'kubernetes-client:master' into master"

e33362f

This reverts commit 638abe2, reversing changes made to 6b85a93.

Changes made in configuration.py to accept environmental variables #2390

Are you sure you want to change the base?

Changes made in configuration.py to accept environmental variables #2390

Uh oh!

Conversation

p172913 commented Apr 29, 2025

Uh oh!

k8s-ci-robot commented Apr 29, 2025

Uh oh!

p172913 commented Apr 29, 2025

Uh oh!

p172913 commented May 1, 2025

Uh oh!

p172913 commented May 5, 2025

Uh oh!

roycaihw commented May 7, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented May 17, 2025

Uh oh!

roycaihw left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

p172913 May 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

p172913 May 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

p172913 commented May 28, 2025

Uh oh!

p172913 commented May 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

p172913 May 30, 2025 •

edited

Loading

p172913 May 28, 2025 •

edited

Loading

p172913 commented May 28, 2025 •

edited

Loading